Fix Units, Add LandLab Mesh, Create Examples#23
Fix Units, Add LandLab Mesh, Create Examples#23danieldouglas92 wants to merge 4 commits intolandlab-aspect:landlab-aspectfrom
Conversation
danieldouglas92
force-pushed
the
landlab-aspect-coupling
branch
from
a3ee432 to
490e5b6
Compare
|
Thanks and very nice examples! I think both questions are something we should discuss first, maybe at the next meeting. |
I disagree. There are many options inside Landlab for defining the mesh (types, spacing, arrangement for hex grids, etc.). It will be very difficult to expose all options inside the .prm file. That is without considering everything that needs to be done on the Landlab side once we allow MPI parallel tasks (the grid or a coarser grid needs to be partitioned). I would suggest that we send information about the ASPECT grid to Landlab (min xyz, max xyz, mesh size) and make the decision there. |
|
Yes, I tend to agree with @tjhei. I think he's right that the mesh should just be defined in a Python script. If you want to run consistency checks, write the interface so that the Python script we execute has a single function that creates the mesh object, and then we can run a separate function that checks for consistency with the ASPECT mesh. I would much rather do these kinds of checks on the Landlab and ASPECT mesh objects, than on the level of parameters for which we have to interpret what Landlab will do with them when turning things into a mesh. |
bangerth
left a comment
bangerth
left a comment
There was a problem hiding this comment.
As for the units: I agree that there is an issue, but I think we should solve it in a different way.
ASPECT does not impose a unit system. It just wants that if you choose one set of units for one parameter, that you use the same for all other parameters. One could work with lengths in feet and time in years, but then you'd have to enter the viscosity not in Pa*s but in pound-force / feet^2 * years, which requires some mental math because the values will not be in the typical range of 1e19-1e24. But one could.
Landlab apparently has the same mental model: Choose your own units, just be consistent.
But in practice, everyone who uses ASPECT for "real" things uses the SI system. And I assume that everyone who wants to use Landlab for "real" things will also use the SI system, just using years instead of seconds. I have no problem making this an assumption for the coupling: Whenever we have to pass values between the two codes, we convert from seconds to years wherever that is necessary. Since neither code has a way of exporting what units it uses, we cannot check that things are correct. That makes the coupling one that works by convention. I'm ok with that. We will put that into the documentation for a future cookbook to make it clear that the coupling assumes this convention.
| double hillslope_diffusion_coefficient; | ||
| double stream_power_erodibility_coefficient; |
There was a problem hiding this comment.
Here, too, I think there are a million Landlab parameters one would eventually have to duplicate. This doesn't seem scalable to me. The better place is to just set these parameters in the .py file.
3 participants
There's three commits in this PR. I'm not convinced everything in here is the "right" way to do things, but this is just what I came up with a couple weeks ago after Timo got things working.
The first commit adds some new parameters to
Mesh deformation/Landlabwhich allows you to define the landlab mesh in the ASPECT prm file. My idea for this is that it's preferable to define the landlab mesh within ASPECT where you can check within a single file that the geometries for both meshes make sense. Right now it just supports structured rectangular grids, but if this seems like the best approach for initializing the LandLab mesh this could easily be extended to allow for HexModelGrids/Voronoi grids with another input parameter.The second commit fixes the units to all be in SI when passing information between landlab/aspect. The issue is that ASPECT requires everything to eventually be converted to seconds, and so the LandLab parameters (which are typically defined in years), needs to be converted to seconds. The changes in this commit I'm sure are not the best, because everytime you want to use a new LandLab parameter you would have to add the parameter as an ASPECT parameter, which means this is not a flexible approach. However I wanted to push the changes anyway just so we can discuss the best way to deal this here.
I think that all of the LandLab parameters should be handled in the .py files, but the motivation for these changes is that it isn't clear that the LandLab parameters need to be in SI units (in the LandLab documentation they state that an advantage of LandLab is that you can use whatever units you want). My thoughts now is that the only useful parameter in this commit is the "Define LandLab parameters using years" parameter, which is set to be unspecified by default, so the user MUST manually set this parameter when running ASPECT with LandLab. I think that this will give the user the information required that they must be careful about units in the LandLab script, and then from there it is up to them to make sure that the parameters they define in the .py scripts are correct. Definitely looking forward to hearing your thoughts on this @bangerth @tjhei
below is lateral_advection.prm
below is vertical_advection.prm
